Skip to content

🔎 fix: Name the Skipped Skill File in the Console Log - #17877

Open
berry-13 wants to merge 2 commits into
devfrom
berry-13/deployment-skill-console-file-name
Open

berry-13 wants to merge 2 commits into
devfrom
berry-13/deployment-skill-console-file-name

Conversation

@berry-13

@berry-13 berry-13 commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

#17876 stopped a bundled file removed from a running deployment skill from aborting the batch upload, but the skipped file's name still does not reach the default console. It passes relativePath as log metadata, and the default console format prints only the message and request context, so Docker stdout shows Failed to get stream: with no file name. It only appears in the JSON file log or with CONSOLE_JSON or DEBUG_CONSOLE on.

The name now goes in the message, matching the neighbouring No download stream for "..." line. This also adds end-to-end coverage of the removed-file path against the fake Code API, which is how the gap was found.

Related to #17846, which can be closed by hand once this merges.

Type of change

  • Bug fix
  • Tests / tooling / CI

Testing

Tested environments/configuration:

Mock e2e harness with the fake Code API, on desktop light, desktop dark and mobile. A fixture deployment skill has one bundled file removed while the server runs and restored afterwards.

Automated tests:

  • npx jest src/skills/__tests__/deployment.test.ts src/agents/skillFiles.spec.ts: 64 of 64 pass, with the log assertion updated to the new message.
  • Added e2e/specs/mock/scenarios/deployment-skill-removed-file.spec.ts: the remaining files reach the Code API without the removed one, and a raw download answers with a fixed error body that names no path. Both pass in all three projects.
  • With the 📂 fix: Reject Missing Deployment Skill Files Before Upload #17876 stream change reversed, both scenarios fail: the fake Code API receives no files, and the download answers Error streaming file.
  • The backend log from the passing run shows Failed to get stream for "b-removed.txt" on the default console.

Screenshots / recordings

No user-facing change

Risk / compatibility

Anything matching the exact old log text needs updating. The new fixture skill is not always-apply, but it does appear in skill lists in the mock harness. A skipped file still does not fail the prime, as described in #17876.

Checklist

  • I reviewed my own changes
  • Relevant tests have been added or updated
  • The change does not introduce new warnings or errors
  • Required documentation PR: N/A

The default console format prints the message and request context but drops metadata, so a relativePath passed as metadata only reached the JSON file log.
@berry-13 berry-13 added 🐛 bug Something isn't working 🔧 fix Bug fixes and code patches 🧪 tests Unit/integration test updates labels Nov 6, 2026
Copilot AI balanced review requested due to automatic review settings November 6, 2026 23:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-06T23:55:37.374946Z 87c7611 PR opened
🔒 Security Review ✅ Completed 2026-11-06T23:58:45.428004Z 87c7611 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working 🔧 fix Bug fixes and code patches 🧪 tests Unit/integration test updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants